Skip to content

fix(metadata): compute the history retention cutoff on one calendar, not two - #16060

Merged
zhuangjianguo merged 1 commit into
mainfrom
claude/issue-15824-history-cleanup-utc-cutoff
Sep 6, 2026
Merged

fix(metadata): compute the history retention cutoff on one calendar, not two#16060
zhuangjianguo merged 1 commit into
mainfrom
claude/issue-15824-history-cleanup-utc-cutoff

Conversation

@claude

@claude claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #15824

The defect

packages/metadata/src/utils/history-cleanup.ts built its retention cutoff on the local calendar and rendered it on UTC:

const cutoffDate = new Date();
cutoffDate.setDate(cutoffDate.getDate() - this.policy.maxAgeDays);   // LOCAL calendar
const cutoffISO = cutoffDate.toISOString();                          // UTC rendering
const filter = { recorded_at: { $lt: cutoffISO } };                  // a DELETE filter

setDate preserves wall-clock time, so stepping the local calendar back n days moves the instant by exactly n × 24h only while every local day in the window is 24 hours long. When the window straddles a DST transition it is 23 hours (spring-forward) or 25 (fall-back), and the cutoff instant slips by the size of that transition — one hour in most zones, thirty minutes on Lord Howe Island.

Both sites, one change

The byte-identical spelling appeared twice. Line numbers re-derived at merge base e1d4f9e3f:

Site Method Line at e1d4f9e3f What the cutoff feeds
delete runCleanup() :96 bulkDeleteByFilter — a real $lt DELETE
preview getCleanupStats() :259 driver.count — the previewed row count

Both now spell it cutoffDate.setUTCDate(cutoffDate.getUTCDate() - this.policy.maxAgeDays).

Measured blast radius

The card describes the error as "twice a year". That is exact for maxAgeDays: 1, and an understatement above it: the window only has to straddle a transition, with no second condition about crossing a UTC midnight, so the exposure grows with the retention period. Measured over a 16-zone × 366-day × 48-half-hour sweep of 2026, as the fraction of instants at which the old spelling produced a wrong cutoff:

zone maxAgeDays: 1 30 90 180
America/New_York 0.6% 16.4% 49.7% 69.4%
Europe/Berlin 0.6% 16.4% 49.7% 84.7%
Australia/Sydney 0.6% 16.4% 49.6% 98.9%
Pacific/Auckland 0.6% 16.4% 49.2% 96.2%
UTC, Asia/Shanghai, Asia/Kolkata, Australia/Perth 0.0% 0.0% 0.0% 0.0%

That bottom row is why nothing in CI has ever gone red on this, and why the pin below cannot be written to run only at TZ=UTC.

The pin

packages/metadata/src/utils/history-cleanup-dst.test.ts — 27 tests, of which 16 are MEASURED DST cells, drawn from a 12-zone × 366-day × 48-half-hour × 7-maxAgeDays sweep of 2026 that found 347,539 disagreeing combinations. Both hemispheres, both transition directions, maxAgeDays from 1 to 180, three zones whose standard offset is not a whole hour (America/St_Johns −03:30, Australia/Adelaide +09:30, Pacific/Chatham +12:45), and one whose transition is not a whole hour (Australia/Lord_Howe, ±30 minutes) so a whole-hour assumption cannot hide in the fix.

Each cell fakes both halves of the environment (process.env.TZ plus a frozen clock) and carries an inline control that asserts the old mixed spelling disagrees there — evaluated before the assertion about the subject. Without it a green run would be ambiguous between "the fix works" and "these instants were not in a transition window", the second being the failure mode that hid the defect. Every cell asserts both call sites, against an oracle that is the definition of the window (instant − maxAgeDays × 86_400_000) rather than a re-implementation of the fix.

Ablation

The mutation was proven on disk before the measurement and the restore proven after, in one shell:

ABLATE: HEAD blob = c064158a6e6425f593855fbae3f025fe812928da
ABLATE: after mutation — fixed-spelling count=0 (want 0), old-spelling count=2 (want 2)
ABLATE: mutated blob = 3a9981195d408195bf22987879f4ad17f8e0361b
ABLATE: MUTATION PROVEN ON DISK
ABLATE: vitest exit under mutation = 1 (want non-zero)
    →  Tests  19 failed | 8 passed (27)
ABLATE: restored blob = c064158a6e6425f593855fbae3f025fe812928da
ABLATE: git diff HEAD -- FILE exit = 0 (want 0)
ABLATE: RESTORE PROVEN (blob == HEAD blob AND git diff HEAD empty)

All 16 measured cells reddened, plus the timezone-invariance fence and both filter-scoping fences. The 8 that stayed green are the ones that do not read the subject's spelling — the cell-table meta-assertions, the non-DST fences and the TZ-restoration fence — which is what they are for.

Fences — what this deliberately does NOT do

  • It does not make retention timezone-aware. The card fences that off explicitly and it needs its own ruling. The pin asserts the opposite: for one instant and one maxAgeDays, the cutoff is now identical in every zone, DST-observing or not.
  • It does not chase the preview-vs-delete disagreement. The two sites read new Date() independently, so they can straddle any boundary for that ordinary reason. That is inherent and this change does not remove it; each site is pinned against the truth separately, at one frozen instant.
  • Nothing else in either filter moved. Pinned: the organization_id scoping, the ADR-0009 executionPinned $nin exclusion, the maxVersions path, and that a policy without maxAgeDays builds no age filter at all.

Published-surface reading

Clause-②: no, both limbs, re-derived rather than assumed — HistoryCleanupManager is exported public API (packages/metadata/src/index.ts:33), so the question is live.

  • Limb 1 (exported symbol or signature moves): no. Built @objectstack/metadata at this head, swapped the one changed source file back to e1d4f9e3f, rebuilt, diffed the emitted dist/index.d.ts: byte-identical, git hash-object = 79b154732b61f586e182e1678c11af085c9cbadd on both sides, diff -u exit 0 / 0 lines. Source restored byte-exact afterwards and proven (blob == HEAD blob, git diff HEAD empty), and the post-restore rebuild reproduced the same d.ts hash.
  • Limb 2 (accept set moves): no. MetadataHistoryRetentionPolicy is untouched; maxAgeDays still accepts exactly z.number().int().positive().optional(). What changes is the cutoff instant — an answer computed from the policy — not which inputs are accepted or rejected.

Verification

All readings below are at d2308c952, with each exit code captured before any pipe.

what result
pnpm --filter @objectstack/metadata test PKG_TEST_EXIT=0Test Files 48 passed (48), Tests 745 passed (745)
pnpm --filter @objectstack/metadata typecheck PKG_TYPECHECK_EXIT=0
typecheck really reads the new files tsc --noEmit --listFiles: 1 hit for history-cleanup-dst.test.ts, 1 for utils/history-cleanup.ts, of 547 files — not a green over source nothing read
derived gate family node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack at d2308c952, provenance line checked; 53 families, all run
gate results 51 exit 0; 2 exit 3 = PREREQUISITE NOT MET, below
pnpm lint (whole repo, eslint . --no-inline-config) LINT_FULL_EXIT=0, no findings — the full scan, not a narrowed one

Gate verdict lines worth quoting:

check-nul-bytes: OK (scanned 7740 text file(s) -- 7740 tracked, 0 untracked-not-ignored;
  skipped 7 binary; no raw ASCII control bytes).
OK: 27 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
check-test-source-alias OK — 72 packages with tests scanned; 61 registered as still resolving a
  workspace dep through `dist/`; 49 published subpath(s) resolved through every alias table.
✓ No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added).
✓ This diff introduces no `major` bump.
check-dts-closure: 13 built package(s) swept - 74/74 declared declaration file(s) present.

The two exit-3 families are NOT MEASURED, in their own words, and both need a full workspace build that CI performs:

  • pnpm check:dual-build-cjs-loads — self-test passed (93 cases); then PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/.⛔ This is NOT a pass: nothing was measured.
  • pnpm check:type-check-debt — the coverage leg passed (check-type-check-coverage: OK — 75/79 workspace packages type-checked); the --re-measure leg refused: 20 workspace dependenc(ies) of the ledgered packages have no built type entry point on disk⛔ This is NOT a pass and NOT a finding.

Generated by Claude Code

…not two

`HistoryCleanupManager` built its `recorded_at: { $lt: … }` bound with
`setDate(getDate() - maxAgeDays)` — the LOCAL calendar — and rendered it with
`toISOString()` — UTC. `setDate` preserves wall-clock time, so a window that
straddles a DST transition moves the instant by 23h or 25h per day-step instead
of 24h, and the cutoff handed to a DELETE filter slips by the size of that
transition. Both call sites (the delete path in `runCleanup()` and the preview
count in `getCleanupStats()`) carried the byte-identical spelling; both now use
`setUTCDate`/`getUTCDate`.

Pinned by `history-cleanup-dst.test.ts`: 16 MEASURED cells from a 12-zone x
366-day x 48-half-hour x 7-maxAgeDays sweep of 2026, both hemispheres, both
transition directions, three sub-hour standard offsets and one sub-hour
transition (Lord_Howe, 30 minutes) — each cell carrying an inline control that
asserts the old spelling disagrees there, so a green run cannot be mistaken for
"the instants were not in a transition window".

Not in scope, deliberately: making retention timezone-aware, and the ordinary
preview-vs-delete disagreement that comes from the two sites reading
`new Date()` independently.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions github-actions Bot added the size/m label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 53cbad9f75572600ef43bb2a18071633fd6c0f68packageMentionDocs.

Which tree this was computed on

This run read content/docs from afd7002674f16d3ac0ee3bb3431b8e8c6db90156 — the merge of head d2308c952989b31814c2665b0b824b31891dadd0 into base 53cbad9f75572600ef43bb2a18071633fd6c0f68, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin afd7002674f16d3ac0ee3bb3431b8e8c6db90156 && git checkout afd7002674f16d3ac0ee3bb3431b8e8c6db90156
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 53cbad9f75572600ef43bb2a18071633fd6c0f68 d2308c952989b31814c2665b0b824b31891dadd0 && git checkout -B drift-repro 53cbad9f75572600ef43bb2a18071633fd6c0f68 && git merge --no-ff d2308c952989b31814c2665b0b824b31891dadd0

node scripts/docs-audit/affected-docs.mjs --json 53cbad9f75572600ef43bb2a18071633fd6c0f68

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026
@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 5, 2026 22:57
@zhuangjianguo
zhuangjianguo added this pull request to the merge queue Sep 5, 2026
Merged via the queue into main with commit 281bf0d Sep 6, 2026
35 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-15824-history-cleanup-utc-cutoff branch September 6, 2026 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HistoryCleanupManager mixes two calendars on the retention cutoff — the age-based delete window slips one hour across a DST transition

2 participants